tests/unit/sdk/test_utils.py (1)

99-104: Optional: expand coverage for edge cases (both None, nested dict with None, and conflicts)

Consider adding a few more assertions to lock in behavior and guard regressions:

• Both sides None keeps None
• Nested dict vs None keeps existing nested structure
• Conflicting non-None scalars raise

For example:

def test_deep_merge_dict_none_additional_cases():
    # both None -> keep None
    assert deep_merge_dict({"k": None}, {"k": None}) == {"k": None}

    # nested dict vs None -> keep existing dict
    a = {"obj": {"sub": 1}}
    b = {"obj": None}
    assert deep_merge_dict(a, b) == {"obj": {"sub": 1}}

    # conflict on non-None scalar -> raises
    with pytest.raises(ValueError):
        deep_merge_dict({"k": "a"}, {"k": "b"})

infrahub_sdk/utils.py (4)

148-157: Optional: short-circuit equality and None-on-right before dict/list handling

A small reordering can avoid unnecessary recursion/list-processing when values are equal or when b_val is None. This keeps behavior identical but saves work in common cases.

Apply within this block:

-            if isinstance(a_val, dict) and isinstance(b_val, dict):
-                deep_merge_dict(a_val, b_val, path + [str(key)])
-            elif isinstance(a_val, list) and isinstance(b_val, list):
-                # Merge lists
-                # Cannot use compare_list because list of dicts won't work (dict not hashable)
-                dicta[key] = [i for i in a_val if i not in b_val] + b_val
-            elif a_val is None and b_val is not None:
-                dicta[key] = b_val
-            elif a_val == b_val or (a_val is not None and b_val is None):
-                continue
+            if a_val == b_val or (a_val is not None and b_val is None):
+                continue
+            elif isinstance(a_val, dict) and isinstance(b_val, dict):
+                deep_merge_dict(a_val, b_val, path + [str(key)])
+            elif isinstance(a_val, list) and isinstance(b_val, list):
+                # Merge lists
+                # Cannot use compare_list because list of dicts won't work (dict not hashable)
+                dicta[key] = [i for i in a_val if i not in b_val] + b_val
+            elif a_val is None and b_val is not None:
+                dicta[key] = b_val
             else:
                 raise ValueError("Conflict at %s" % ".".join(path + [str(key)]))

---

154-157: None-handling semantics look correct; consider documenting explicitly

Behavior now is:

• If a is None and b is not None: take b
• If b is None (and a is not None): keep a
• If equal: keep a
• Otherwise, merge dicts/lists or raise conflict

Recommend updating the function docstring to call out in-place merge semantics and the None rules to prevent misuse.

Here’s suggested docstring text (outside the changed lines):

def deep_merge_dict(dicta: dict, dictb: dict, path: list[str] | None = None) -> dict:
    """Deep-merge dictb into dicta, modifying dicta in place and returning it.

    Rules:
    - Dict vs dict: recursively deep-merge.
    - List vs list: keep items unique, preserve order: items from dicta not in dictb, then all items from dictb.
    - None handling (per key):
      * dicta[key] is None and dictb[key] is not None -> take dictb[key].
      * dictb[key] is None (and dicta[key] is not None) -> keep dicta[key].
      * equal values -> keep dicta[key].
    - Any other non-equal, non-None type/value mismatch -> raise ValueError with a path to the conflict.
    """

---

153-153: FYI: list merge is O(n*m); acceptable, but watch for very large lists

The membership check i not in b_val leads to quadratic behavior. If you expect very large lists of hashables, consider a specialized fast-path (e.g., precompute set_b = set(b_val) when all elements are hashable) and fall back to the current approach for unhashables (dicts).

---

161-161: Nit: reuse b_val alias for consistency

dicta[key] = dictb[key] can become dicta[key] = b_val to match the alias pattern introduced above.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro